From 6f5f9775347a3a77bdbd287953a91b0ba7f15a9e Mon Sep 17 00:00:00 2001 From: Jyrki Gadinger Date: Mon, 31 Mar 2025 11:48:22 +0200 Subject: [PATCH] fix: try to correct mtime on upsyncs Files with a modification time of less than 0 do usually not make sense (and afaik the server doesn't accept them either). --> attempt to update the modification time to _Time.now_ while propagating side note: I ran into this because KArchive/Ark(?) didn't consider the extra time attributes on entries for a certain zip file, so it instead used the standard time value of each zip entry which was set to <= 1980 for files and < 1970 for directories... Signed-off-by: Jyrki Gadinger --- src/libsync/bulkpropagatorjob.cpp | 35 ++++++++++++----- src/libsync/discovery.cpp | 1 + src/libsync/propagateupload.cpp | 11 +++++- test/syncenginetestutils.cpp | 3 +- test/testsyncengine.cpp | 65 +++++++++++++++++++++++++++++++ test/testsyncvirtualfiles.cpp | 23 +++++++++-- 6 files changed, 122 insertions(+), 16 deletions(-) diff --git a/src/libsync/bulkpropagatorjob.cpp b/src/libsync/bulkpropagatorjob.cpp index 637e46c9a..942605f18 100644 --- a/src/libsync/bulkpropagatorjob.cpp +++ b/src/libsync/bulkpropagatorjob.cpp @@ -171,10 +171,17 @@ void BulkPropagatorJob::doStartUpload(SyncFileItemPtr item, item->_modtime = FileSystem::getModTime(newFilePathAbsolute); if (item->_modtime <= 0) { - _pendingChecksumFiles.remove(item->_file); - slotOnErrorStartFolderUnlock(item, SyncFileItem::NormalError, tr("File %1 has invalid modified time. Do not upload to the server.").arg(QDir::toNativeSeparators(item->_file)), ErrorCategory::GenericError); - checkPropagationIsDone(); - return; + const auto now = QDateTime::currentSecsSinceEpoch(); + qCInfo(lcPropagateUpload) << "File" << item->_file << "has invalid modification time of" << item->_modtime << "-- trying to update it to" << now; + if (FileSystem::setModTime(newFilePathAbsolute, now)) { + item->_modtime = now; + } else { + qCWarning(lcPropagateUpload) << "Could not update modification time for" << item->_file; + _pendingChecksumFiles.remove(item->_file); + slotOnErrorStartFolderUnlock(item, SyncFileItem::NormalError, tr("File %1 has invalid modified time. Do not upload to the server.").arg(QDir::toNativeSeparators(item->_file)), ErrorCategory::GenericError); + checkPropagationIsDone(); + return; + } } } @@ -298,18 +305,26 @@ void BulkPropagatorJob::slotStartUpload(SyncFileItemPtr item, return; } - const auto prevModtime = item->_modtime; // the _item value was set in PropagateUploadFile::start() + const auto prevModtime = item->_modtime; // the item value was set in PropagateUploadFile::start() // but a potential checksum calculation could have taken some time during which the file could // have been changed again, so better check again here. item->_modtime = FileSystem::getModTime(originalFilePath); + qCDebug(lcPropagateUpload) << "fullFilePath" << fullFilePath << "originalFilePath" << originalFilePath << "prevModtime" << prevModtime << "item->_modtime" << item->_modtime; if (item->_modtime <= 0) { - _pendingChecksumFiles.remove(item->_file); - slotOnErrorStartFolderUnlock(item, SyncFileItem::NormalError, tr("File %1 has invalid modification time. Do not upload to the server.").arg(QDir::toNativeSeparators(item->_file)), ErrorCategory::GenericError); - checkPropagationIsDone(); - return; + const auto now = QDateTime::currentSecsSinceEpoch(); + qCInfo(lcPropagateUpload) << "File" << item->_file << "has invalid modification time of" << item->_modtime << "-- trying to update it to" << now; + if (FileSystem::setModTime(originalFilePath, now)) { + item->_modtime = now; + } else { + qCWarning(lcPropagateUpload) << "Could not update modification time for" << item->_file; + _pendingChecksumFiles.remove(item->_file); + slotOnErrorStartFolderUnlock(item, SyncFileItem::NormalError, tr("File %1 has invalid modification time. Do not upload to the server.").arg(QDir::toNativeSeparators(item->_file)), ErrorCategory::GenericError); + checkPropagationIsDone(); + return; + } } - if (prevModtime != item->_modtime) { + if (prevModtime > 0 && prevModtime != item->_modtime) { propagator()->_anotherSyncNeeded = true; _pendingChecksumFiles.remove(item->_file); diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp index 9939d4473..4c0dfa151 100644 --- a/src/libsync/discovery.cpp +++ b/src/libsync/discovery.cpp @@ -1121,6 +1121,7 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo( } if ((item->_direction == SyncFileItem::Down || item->_instruction == CSYNC_INSTRUCTION_CONFLICT || item->_instruction == CSYNC_INSTRUCTION_NEW || item->_instruction == CSYNC_INSTRUCTION_SYNC) && + item->_direction != SyncFileItem::Up && (item->_modtime <= 0 || item->_modtime >= 0xFFFFFFFF)) { item->_instruction = CSYNC_INSTRUCTION_ERROR; item->_errorString = tr("Cannot sync due to invalid modification time"); diff --git a/src/libsync/propagateupload.cpp b/src/libsync/propagateupload.cpp index 4946f05e9..2bac80f17 100644 --- a/src/libsync/propagateupload.cpp +++ b/src/libsync/propagateupload.cpp @@ -324,8 +324,15 @@ void PropagateUploadFileCommon::slotComputeContentChecksum() // probably temporary one. _item->_modtime = FileSystem::getModTime(filePath); if (_item->_modtime <= 0) { - slotOnErrorStartFolderUnlock(SyncFileItem::NormalError, tr("File %1 has invalid modification time. Do not upload to the server.").arg(QDir::toNativeSeparators(_item->_file))); - return; + const auto now = QDateTime::currentSecsSinceEpoch(); + qCInfo(lcPropagateUpload) << "File" << _item->_file << "has invalid modification time of" << _item->_modtime << "-- trying to update it to" << now; + if (FileSystem::setModTime(filePath, now)) { + _item->_modtime = now; + } else { + qCWarning(lcPropagateUpload) << "Could not update modification time for" << _item->_file; + slotOnErrorStartFolderUnlock(SyncFileItem::NormalError, tr("File %1 has invalid modification time. Do not upload to the server.").arg(QDir::toNativeSeparators(_item->_file))); + return; + } } const QByteArray checksumType = propagator()->account()->capabilities().preferredUploadChecksumType(); diff --git a/test/syncenginetestutils.cpp b/test/syncenginetestutils.cpp index 8eac8a06e..507fcd2ee 100644 --- a/test/syncenginetestutils.cpp +++ b/test/syncenginetestutils.cpp @@ -535,6 +535,7 @@ FakePutMultiFileReply::FakePutMultiFileReply(FileInfo &remoteRootFileInfo, QNetw QVector FakePutMultiFileReply::performMultiPart(FileInfo &remoteRootFileInfo, const QNetworkRequest &request, const QByteArray &putPayload, const QString &contentType) { + Q_UNUSED(request) QVector result; auto stringPutPayload = QString::fromUtf8(putPayload); @@ -564,7 +565,7 @@ QVector FakePutMultiFileReply::performMultiPart(FileInfo &remoteRoot // Assume that the file is filled with the same character fileInfo = remoteRootFileInfo.create(fileName, onePartBody.size(), onePartBody.at(0).toLatin1()); } - fileInfo->lastModified = OCC::Utility::qDateTimeFromTime_t(request.rawHeader("x-oc-mtime").toLongLong()); + fileInfo->lastModified = OCC::Utility::qDateTimeFromTime_t(modtime); remoteRootFileInfo.find(fileName, /*invalidateEtags=*/true); result.push_back(fileInfo); } diff --git a/test/testsyncengine.cpp b/test/testsyncengine.cpp index b2d721aaa..d1d182c82 100644 --- a/test/testsyncengine.cpp +++ b/test/testsyncengine.cpp @@ -1333,6 +1333,71 @@ private slots: QCOMPARE(fakeFolder.currentRemoteState(), expectedState); } + void testLocalInvalidMtimeCorrection() + { + const auto INVALID_MTIME = QDateTime::fromSecsSinceEpoch(0); + const auto RECENT_MTIME = QDateTime::fromSecsSinceEpoch(1743004783); // 2025-03-26T16:59:43+0100 + + FakeFolder fakeFolder{FileInfo{}}; + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + + fakeFolder.localModifier().insert(QStringLiteral("invalid")); + fakeFolder.localModifier().setModTime("invalid", INVALID_MTIME); + fakeFolder.localModifier().insert(QStringLiteral("recent")); + fakeFolder.localModifier().setModTime("recent", RECENT_MTIME); + + QVERIFY(fakeFolder.syncOnce()); + + // "invalid" file had a mtime of 0, so it's been updated to the current time during testing + const auto currentMtime = fakeFolder.currentLocalState().find("invalid")->lastModified; + QCOMPARE_GT(currentMtime, RECENT_MTIME); + QCOMPARE_GT(fakeFolder.currentRemoteState().find("invalid")->lastModified, RECENT_MTIME); + + // "recent" file had a mtime of RECENT_MTIME, so it shouldn't have been changed + QCOMPARE(fakeFolder.currentLocalState().find("recent")->lastModified, RECENT_MTIME); + QCOMPARE(fakeFolder.currentRemoteState().find("recent")->lastModified, RECENT_MTIME); + + QVERIFY(fakeFolder.syncOnce()); + + // verify that the mtime of "invalid" hasn't changed since the last sync that fixed it + QCOMPARE(fakeFolder.currentLocalState().find("invalid")->lastModified, currentMtime); + + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + } + + void testLocalInvalidMtimeCorrectionBulkUpload() + { + const auto INVALID_MTIME = QDateTime::fromSecsSinceEpoch(0); + const auto RECENT_MTIME = QDateTime::fromSecsSinceEpoch(1743004783); // 2025-03-26T16:59:43+0100 + + FakeFolder fakeFolder{FileInfo{}}; + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + fakeFolder.syncEngine().account()->setCapabilities({ { "dav", QVariantMap{ {"bulkupload", "1.0"} } } }); + + fakeFolder.localModifier().insert(QStringLiteral("invalid")); + fakeFolder.localModifier().setModTime("invalid", INVALID_MTIME); + fakeFolder.localModifier().insert(QStringLiteral("recent")); + fakeFolder.localModifier().setModTime("recent", RECENT_MTIME); + + QVERIFY(fakeFolder.syncOnce()); // this will use the BulkPropagatorJob + + // "invalid" file had a mtime of 0, so it's been updated to the current time during testing + const auto currentMtime = fakeFolder.currentLocalState().find("invalid")->lastModified; + QCOMPARE_GT(currentMtime, RECENT_MTIME); + QCOMPARE_GT(fakeFolder.currentRemoteState().find("invalid")->lastModified, RECENT_MTIME); + + // "recent" file had a mtime of RECENT_MTIME, so it shouldn't have been changed + QCOMPARE(fakeFolder.currentLocalState().find("recent")->lastModified, RECENT_MTIME); + QCOMPARE(fakeFolder.currentRemoteState().find("recent")->lastModified, RECENT_MTIME); + + QVERIFY(fakeFolder.syncOnce()); // this will not propagate anything + + // verify that the mtime of "invalid" hasn't changed since the last sync that fixed it + QCOMPARE(fakeFolder.currentLocalState().find("invalid")->lastModified, currentMtime); + + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + } + void testServerUpdatingMTimeShouldNotCreateConflicts() { constexpr auto testFile = "test.txt"; diff --git a/test/testsyncvirtualfiles.cpp b/test/testsyncvirtualfiles.cpp index f6281ab38..03e743ec3 100644 --- a/test/testsyncvirtualfiles.cpp +++ b/test/testsyncvirtualfiles.cpp @@ -1751,6 +1751,10 @@ private slots: return {}; }; + const auto lastModified = [&](const QString &path) -> qint64 { + return fakeFolder.currentLocalState().find(path)->lastModified.toSecsSinceEpoch(); + }; + fakeFolder.localModifier().insert(fooFileRootFolder); fakeFolder.localModifier().insert(barFileRootFolder); fakeFolder.localModifier().mkdir(QStringLiteral("subfolder")); @@ -1765,24 +1769,33 @@ private slots: fakeFolder.scheduleSync(); fakeFolder.execUntilBeforePropagation(); - QCOMPARE(checkStatus(), SyncFileStatus::StatusError); + QCOMPARE(checkStatus(), SyncFileStatus::StatusSync); fakeFolder.execUntilFinished(); + // ensure mtime has changed after the sync + QCOMPARE_GT(lastModified(barFileAaaSubFolder), CURRENT_MTIME); + fakeFolder.localModifier().setModTime(barFileAaaSubFolder, QDateTime::fromSecsSinceEpoch(CURRENT_MTIME)); QVERIFY(fakeFolder.syncOnce()); + // ensure mtime is now CURRENT_MTIME + QCOMPARE(lastModified(barFileAaaSubFolder), CURRENT_MTIME); + fakeFolder.localModifier().appendByte(barFileAaaSubFolder); fakeFolder.localModifier().setModTime(barFileAaaSubFolder, QDateTime::fromSecsSinceEpoch(INVALID_MTIME1)); fakeFolder.scheduleSync(); fakeFolder.execUntilBeforePropagation(); - QCOMPARE(checkStatus(), SyncFileStatus::StatusError); + QCOMPARE(checkStatus(), SyncFileStatus::StatusSync); fakeFolder.execUntilFinished(); + // ensure mtime has changed after the sync + QCOMPARE_GT(lastModified(barFileAaaSubFolder), CURRENT_MTIME); + fakeFolder.localModifier().setModTime(barFileAaaSubFolder, QDateTime::fromSecsSinceEpoch(CURRENT_MTIME)); QVERIFY(fakeFolder.syncOnce()); @@ -1793,7 +1806,11 @@ private slots: fakeFolder.scheduleSync(); fakeFolder.execUntilBeforePropagation(); - QCOMPARE(checkStatus(), SyncFileStatus::StatusError); + QCOMPARE(checkStatus(), SyncFileStatus::StatusSync); + + // the server only considers an mtime of 0-86400 (1d) as invalid, so this is fine + // see also: apps/dav/lib/Connector/Sabre/MtimeSanitizer.php + QCOMPARE(lastModified(barFileAaaSubFolder), INVALID_MTIME2); fakeFolder.execUntilFinished(); } -- 2.30.2